feat: add sort toggle for RHS items (Created vs Updated)#1025
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds sidebar sorting by created or updated time, introduces a dedicated dropdown component, and wires a dev preview with webpack dev-server support. It also adds planning documents for the PR-fix workflow. ChangesSidebar sort feature
Planning documents
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
avasconcelos114
left a comment
avasconcelos114
left a comment
There was a problem hiding this comment.
Hello and thank you for the contribution! 🎉 :)
Before diving into reviewing the implementation I tested a build with your PR and noticed that the style of the select will need to be adjusted to match what the issue had it
Refer to the comment in #242 (comment) to view the Figma prototype and let me know if you have any questions, thanks again!
avasconcelos114
added
2: Dev Review
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
|
Hi @avasconcelos114, I've addressed your review feedback in commit 394e874:
The dropdown is |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webapp/src/components/sidebar_right/sidebar_right.jsx`:
- Around line 223-235: The section header layout is missing a flex row, so the
new sort dropdown will stack under the title instead of sitting beside it.
Update the style returned by makeStyleFromTheme for sectionHeader in
sidebar_right.jsx to use a horizontal flex layout (while keeping the existing
padding), so the title link and sortDropdownContainer render on the same row.
- Around line 161-174: The unread-items sort logic in sidebar_right.jsx is using
created_at as the default for RHSStates.UNREADS, but those notification threads
only have updated_at, so the “Sort: Created” selection ends up comparing
undefined dates. Update the sorting branch around the sortedItems and
sortOptions/selectedSortOption logic to fall back to updated_at for unread
items, or hide/disable the Created option in this view so the selected label
matches the actual sort behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 55e1de93-0482-46c7-babd-2c1e4f0c3c26
📒 Files selected for processing (1)
webapp/src/components/sidebar_right/sidebar_right.jsx
|
Addressed both CodeRabbit issues in 5d55c5d:
|
|
Hello again! Thank you for the revisions on the last batch of changes, I've confirmed that the sorting works great functionally :) Though there is still quite a bit of discrepancy between the style in your implementation compared to the design created in Figma, but after this we should be good to officially QA and merge!
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webapp/src/components/sidebar_right/sort_dropdown.jsx`:
- Around line 252-267: The option style object in sort_dropdown.jsx uses nested
:hover and :focus pseudo-selectors, but the React style prop ignores them.
Update the dropdown option rendering to use a styling approach that supports
interactive states, such as state-driven
onMouseEnter/onMouseLeave/onFocus/onBlur handling in the option component or a
CSS-in-JS solution already used in the codebase. Keep the existing option style
symbol and ensure the hover/focus visuals are applied through the component
logic rather than inline pseudo-selectors.
- Line 22: SortDropdown is using getStyle(theme) but the component is currently
rendered without a theme prop, so theme.centerChannelColor can be undefined at
render time. Update the SortDropdown usage so the parent passes the existing
theme value through, and verify the SortDropdown component and its PropTypes
definition stay aligned with the required theme prop.
- Around line 197-215: The trigger style in sort_dropdown’s dropdown
configuration removes keyboard focus visibility by setting outline to none, so
update the trigger styling to preserve a visible focus indicator. Adjust the
trigger style in the sort_dropdown component to use a theme-based focus ring or
state-driven focus style for the trigger button, and avoid relying on :focus in
inline styles so the focus indicator remains visible when the sort trigger is
keyboard-focused.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 955b08ec-3017-41b5-a5ec-86f540b7cb67
📒 Files selected for processing (2)
webapp/src/components/sidebar_right/sidebar_right.jsxwebapp/src/components/sidebar_right/sort_dropdown.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- webapp/src/components/sidebar_right/sidebar_right.jsx
rafaumeu
force-pushed
the
feature/sort-rhs-242
branch
from
c714531 to
1db479e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
webapp/src/dev.jsx (1)
112-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThird dropdown shares state with the first.
The keyboard-navigation test dropdown (lines 112-118) reuses the same
selected/setSelectedstate as the default dropdown (lines 50-56). Interacting with one updates the other, which may be confusing during manual testing. Consider a separate state variable if independent behavior is intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webapp/src/dev.jsx` around lines 112 - 118, The keyboard-test SortDropdown is sharing the same selected/setSelected state as the default SortDropdown, so interactions are coupled. In dev.jsx, give the keyboard test dropdown its own state variable and updater instead of reusing the existing selected state, and wire that new state through the SortDropdown props so the two dropdowns behave independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webapp/src/components/sidebar_right/sort_dropdown.jsx`:
- Around line 50-52: The dropdown close handler in SortDropdown should clear
both the open flag and any lingering hover state. Update the handleClose method
in sort_dropdown.jsx so that hoveredOption is reset when isOpen is set to false,
preventing stale highlight state from surviving a dismiss-and-reopen cycle.
- Around line 107-110: The SortDropdown button is using a static accessible name
that hides the current selected sort value. Update the button in SortDropdown to
avoid a fixed aria-label like “Sort items”; either remove the aria-label so the
visible text from the button content is used, or make the label dynamic to
include the current selection from the sort state/value (e.g., the same value
shown in the button text).
In `@webapp/webpack.config.js`:
- Around line 25-31: HtmlWebpackPlugin is being added unconditionally in the
webpack.config.js plugins setup, so production bundling creates an unnecessary
index.html in dist. Update the plugins array in the webpack config to only
include HtmlWebpackPlugin when isDev is true, while keeping webpack.DefinePlugin
active for all builds.
---
Nitpick comments:
In `@webapp/src/dev.jsx`:
- Around line 112-118: The keyboard-test SortDropdown is sharing the same
selected/setSelected state as the default SortDropdown, so interactions are
coupled. In dev.jsx, give the keyboard test dropdown its own state variable and
updater instead of reusing the existing selected state, and wire that new state
through the SortDropdown props so the two dropdowns behave independently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0a960729-af05-4d24-8d78-5ec4140c1da6
⛔ Files ignored due to path filters (3)
webapp/package-lock.jsonis excluded by!**/package-lock.jsonwebapp/sort_dropdown_closed.pngis excluded by!**/*.pngwebapp/sort_dropdown_open.pngis excluded by!**/*.png
📒 Files selected for processing (10)
.planning/mattermost-pr-fixes/AGENTS.md.planning/mattermost-pr-fixes/CONTEXT.md.planning/mattermost-pr-fixes/PLAN.md.planning/mattermost-pr-fixes/SPEC.mdwebapp/package.jsonwebapp/public/index.htmlwebapp/src/components/sidebar_right/sidebar_right.jsxwebapp/src/components/sidebar_right/sort_dropdown.jsxwebapp/src/dev.jsxwebapp/webpack.config.js
🚧 Files skipped from review as they are similar to previous changes (1)
- webapp/src/components/sidebar_right/sidebar_right.jsx
✅ Addressed in commit 24ebe73 — the keyboard-test dropdown now uses its own <SortDropdown
- value={selected}
- onChange={setSelected}
+ value={selected2}
+ onChange={setSelected2}
options={options}
theme={mockTheme}
ariaLabel={'Keyboard test'}
/>Both dropdowns now operate independently. |
Add a SortDropdown component to the RHS sidebar that allows users to toggle sorting between 'Recently Created' and 'Recently Updated'. - SortDropdown: custom accessible dropdown with theme support, keyboard navigation, and click-outside-to-close behavior - SidebarRight: integrates SortDropdown, sorts items by selected criteria, falls back to updated_at for notification threads that lack created_at - Uses makeStyleFromTheme for consistent theme-aware styling Fixes mattermost#242
rafaumeu
force-pushed
the
feature/sort-rhs-242
branch
from
24ebe73 to
a0bb43e
Compare
3 participants
Summary
Adds a sort dropdown to the GitHub plugin RHS sidebar, allowing users to sort items by "Created" (default, existing behavior) or "Updated" date.
Resolves #242
Changes
<select>dropdown in the RHS header with two options: "Sort: Created" and "Sort: Updated"created_atorupdated_atfield (descending)Release Notes
Change Impact: 🟡 Medium
Reasoning: This introduces a new, user-facing right-sidebar sorting control and new client-side ordering logic (including
updated_atfallback behavior), plus a fresh interactive dropdown component with keyboard/ARIA handling. While it doesn’t touch auth/session/data persistence, it can affect rendering and interaction correctness on the RHS across multiple sidebar states.Regression Risk: Moderate — sorting and UI interaction changes are localized to the sidebar components, but date parsing/sorting and dropdown open/close/selection behavior are new paths that could regress in common usage.
QA Recommendation: Moderate manual QA recommended: validate default “Recently Created” and “Recently Updated” ordering for PR/review/unread/assignment contexts, confirm unread thread fallback when
created_atis missing, and test dropdown mouse + keyboard navigation/focus/ARIA across light/dark themes. Skipping manual QA is not advised for the sorting dropdown and ordering behavior.Generated by CodeRabbitAI